Skip to content

refactor(wizard): use computed signal for internal states#1855

Merged
chintankavathia merged 1 commit intomainfrom
refactor/wizard-migrate-to-computed
Apr 20, 2026
Merged

refactor(wizard): use computed signal for internal states#1855
chintankavathia merged 1 commit intomainfrom
refactor/wizard-migrate-to-computed

Conversation

@spliffone
Copy link
Copy Markdown
Member

@spliffone spliffone commented Apr 13, 2026

Use computed signal to calculate and cache the internal wizard step states instead of function call which would be called in each check cycle.


Documentation.
Examples.
Dashboards Demo.
Playwright report.

Coverage Reports:

Code Coverage

@spliffone spliffone requested review from a team as code owners April 13, 2026 07:21
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the SiWizardComponent to utilize Angular signals for derived state, replacing several methods with computed signals that pre-calculate step properties such as states, icons, and ARIA attributes. While this aligns with signal-based state management, the canActivateSteps signal introduces an O(N²) complexity that should be optimized to O(N). Additionally, consolidating the multiple computed signals that iterate over the steps array into a single metadata signal would improve maintainability and reduce redundant allocations.

Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
@spliffone spliffone added the enhancement Topics that make the project better label Apr 13, 2026
@spliffone spliffone added this to the 49.x milestone Apr 13, 2026
Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
@spliffone spliffone force-pushed the refactor/wizard-migrate-to-computed branch from 4820cba to 4d395a0 Compare April 17, 2026 08:09
Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
@spliffone spliffone force-pushed the refactor/wizard-migrate-to-computed branch from a3d67f9 to f386731 Compare April 17, 2026 09:08
@spliffone
Copy link
Copy Markdown
Member Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the SiWizardComponent to use a computed signal for step metadata, optimizing performance by pre-calculating state, accessibility attributes, and icons for each step. This change replaces multiple individual method calls in the template with a unified metadata object. Feedback was provided to add a bounds check when accessing the metadata array in the next method to prevent potential runtime errors from out-of-bounds indices.

Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
@spliffone spliffone force-pushed the refactor/wizard-migrate-to-computed branch 2 times, most recently from fb43ccd to 4f3a17e Compare April 20, 2026 05:02
Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
Comment thread projects/element-ng/wizard/si-wizard.component.ts Outdated
@spliffone spliffone force-pushed the refactor/wizard-migrate-to-computed branch from 4f3a17e to acee226 Compare April 20, 2026 09:46
Copy link
Copy Markdown
Member

@chintankavathia chintankavathia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@chintankavathia chintankavathia added this pull request to the merge queue Apr 20, 2026
Merged via the queue into main with commit cbc7ee8 Apr 20, 2026
11 checks passed
@chintankavathia chintankavathia deleted the refactor/wizard-migrate-to-computed branch April 20, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Topics that make the project better

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants